feat: option to triggering reconciler on all events #2894

csviri · 2025-08-11T10:09:58Z

Adds option to trigger reconciliation on all events.

Signed-off-by: Attila Mészáros [email protected]

csviri · 2025-08-21T14:38:26Z

@metacosm @xstefank I will add a bunch of unit tests, and docs, but the core of it is ready. Pls take a look, thank you.

Signed-off-by: Attila Mészáros <[email protected]>

csviri · 2025-09-17T11:41:03Z

@xstefank @metacosm This feature is ready now, please review it. Happy to give also a tour together in case that is needed. thank you

Signed-off-by: Attila Mészáros <[email protected]>

metacosm · 2025-09-17T12:56:19Z

As mentioned in the issue, I'd rather see this supported via listeners than via yet another feature flag.

csviri · 2025-09-17T14:58:33Z

As mentioned in the issue, I'd rather see this supported via listeners than via yet another feature flag.

The idea behind this is same as controllers work in go, having a separate listernet would not be an eleant solution, basically we just trigger reconiliation for all the events as in go is done by default. This way we don't need an another interface or any other construct.

csviri · 2025-09-17T16:12:51Z

As mentioned in the issue, I'd rather see this supported via listeners than via yet another feature flag.

Also in the past on purpose reduced the number of those interaces, and I think that was the right choice. I'm not that happy about to much feature flags, but I see this the as the better option now, since it is closed to go; feature flags use can much easier see than a new interface. And note that this is nost just about the delete events, but all the events, for example when others have finalizers added to the custom resources, it is marked for deletion, but meanwhile updated (or just on the mar of deletion event) so acutally much harder to capture with an interface. I really think this is a better model just to have to trigger the reconciler on everthing than having separate methods.

metacosm · 2025-09-17T16:22:38Z

Following the go SDK should only been done where it makes sense. I don't think it's a particularly compelling argument for this particular design or the other flags that have been introduced lately.

csviri · 2025-09-17T16:55:27Z

Following the go SDK should only been done where it makes sense. I don't think it's a particularly compelling argument for this particular design or the other flags that have been introduced lately.

That is just a side note regarding to go, I don't see how an interface is a good alterative for that, especailly that we tried to eliminate (and did) all the interfaces for v5. But the main issues I see that, it would be even hard to scope as an interface, how would you define that? which events would trigger that interface?

csviri · 2025-09-17T16:56:14Z

other flags that have been introduced lately

What other flags you mean?

csviri · 2025-09-19T08:54:50Z

@metacosm I was giving more thoughs about this, but basically came to the same conclusion, regarding feature flags / interfaces.
I completely agree that best would be to have those as less as possible. But since we tried to eliminate the interaces for v5 and moving most of it (except Cleaner what is justifiable) into the Reconciler interface as default method. Basically to have this mode it the only viable options is to have it as feature flag, since event it would be harder to define as an interface.

Basically this was to have all events trigger the reconiler functions gives a very good mental model, and a simplistic user interface, that "I get triggered an all events, and can check in what state the primary and secondary resources are and act upon that"

Pls review the PR in depth.

csviri · 2025-09-19T08:55:30Z

other flags that have been introduced lately

For this pls create an issue where we can separately discuss your concerns in detail.

metacosm · 2025-09-23T12:09:42Z

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java

  boolean isNextReconciliationImminent();
+
+  /**
+   * To check if the primary resource is already deleted. This value can be true only if you turn on


This should be enforced in the code.

metacosm · 2025-09-23T12:09:56Z

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java

+  boolean isPrimaryResourceDeleted();
+
+  /**
+   * Check this only if {@link #isPrimaryResourceDeleted()} is true.


Same as above.

metacosm · 2025-09-23T12:12:28Z

...ore/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java

+  public static <P extends HasMetadata> P conflictRetryingPatch(
+      KubernetesClient client,
+      P resource,
+      UnaryOperator<P> unaryOperator,


Please provide proper javadoc for all public methods with appropriate parameter names (e.g. unaryOperator doesn't help understand what is the purpose of this parameter).

metacosm · 2025-09-23T12:13:31Z

...va/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java

  public void onDelete(T resource, boolean b) {
    super.onDelete(resource, b);
-    eventReceived(ResourceAction.DELETED, resource, null);
+    eventReceived(ResourceAction.DELETED, resource, null, b);


Please use a proper parameter name instead of b.

metacosm · 2025-09-23T12:14:37Z

...java/io/javaoperatorsdk/operator/processing/event/source/controller/ResourceDeleteEvent.java

+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.processing.event.ResourceID;
+
+public class ResourceDeleteEvent extends ResourceEvent {


Please add documentation.

metacosm · 2025-09-23T12:16:17Z

...ramework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java


  private void handleMarkedEventForResource(ResourceState state) {
-    if (state.deleteEventPresent()) {
+    if (state.deleteEventPresent() && !triggerOnAllEvent()) {


Extract in a separate method, that can be reused elsewhere.

metacosm · 2025-09-23T12:44:20Z

...ramework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

+            log.debug(
+                "Resource not found in the cache, checking for delete event resource: {}",
+                resourceID);
+            var state = resourceStateManager.get(resourceID);


This should only be done / needed if this is a delete event so the state retrieval should only be done in that case.

metacosm · 2025-09-23T12:45:04Z

...ramework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

+                          .filter(ResourceState::deleteEventPresent)
+                          .map(ResourceState::getLastKnownResource);
+              if (actualResource.isEmpty()) {
+                throw new IllegalStateException("this should not happen");


Please add a proper message here instead.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 11, 2025

csviri linked an issue Aug 11, 2025 that may be closed by this pull request

Option to trigger reconciler on all event #2893

Open

csviri changed the title ~~feat: all-event mode~~ feat: all-event reconcile mode Aug 11, 2025

csviri force-pushed the all-event-mode branch from 48d768c to 6d84d08 Compare August 11, 2025 10:35

csviri changed the base branch from main to next August 11, 2025 10:35

csviri changed the title ~~feat: all-event reconcile mode~~ feat: reconcile-all-event mode Aug 11, 2025

csviri force-pushed the next branch from a6add80 to 9a19dcb Compare August 21, 2025 11:10

csviri force-pushed the all-event-mode branch from 323bb28 to c69472c Compare August 21, 2025 13:39

csviri requested review from metacosm and xstefank August 21, 2025 14:50

csviri force-pushed the all-event-mode branch from c4f8193 to eeff1ed Compare September 1, 2025 10:34

csviri force-pushed the next branch from 72c5400 to e0f8b5b Compare September 2, 2025 11:16

csviri force-pushed the all-event-mode branch from aa171b6 to 8c44cd5 Compare September 2, 2025 11:23

csviri changed the title ~~feat: reconcile-all-event mode~~ feat: allow to propagate all event to reconiler Sep 3, 2025

csviri changed the title ~~feat: allow to propagate all event to reconiler~~ feat: trigger reconciler on all event Sep 4, 2025

csviri changed the title ~~feat: trigger reconciler on all event~~ feat: triggering reconciler on all event Sep 4, 2025

csviri force-pushed the all-event-mode branch from 2e5e220 to 8f69a5e Compare September 4, 2025 12:18

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2025

csviri force-pushed the all-event-mode branch from 0f0cca9 to fd68244 Compare September 16, 2025 08:30

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2025

csviri changed the title ~~feat: triggering reconciler on all event~~ feat: option to triggering reconciler on all events Sep 17, 2025

csviri added 8 commits September 17, 2025 13:00

feat: all-event mode

7e368db

Signed-off-by: Attila Mészáros <[email protected]>

wip

8ca44d2

Signed-off-by: Attila Mészáros <[email protected]>

wip

525fae7

Signed-off-by: Attila Mészáros <[email protected]>

wip

9bcdc84

Signed-off-by: Attila Mészáros <[email protected]>

wip

b1eb9bf

Signed-off-by: Attila Mészáros <[email protected]>

wip

cfbb8b5

Signed-off-by: Attila Mészáros <[email protected]>

wip

c3ad61d

Signed-off-by: Attila Mészáros <[email protected]>

wip

27850ba

Signed-off-by: Attila Mészáros <[email protected]>

csviri added 12 commits September 17, 2025 13:00

fix

923f9bd

Signed-off-by: Attila Mészáros <[email protected]>

wip

f6fa483

Signed-off-by: Attila Mészáros <[email protected]>

test fix

66858dd

Signed-off-by: Attila Mészáros <[email protected]>

wip

167b3c0

Signed-off-by: Attila Mészáros <[email protected]>

wip

eb78dcc

Signed-off-by: Attila Mészáros <[email protected]>

wip

7808fb6

Signed-off-by: Attila Mészáros <[email protected]>

wip

ac1a026

Signed-off-by: Attila Mészáros <[email protected]>

wip

04938f4

Signed-off-by: Attila Mészáros <[email protected]>

wip

c884fb7

Signed-off-by: Attila Mészáros <[email protected]>

docs

92a9e72

Signed-off-by: Attila Mészáros <[email protected]>

javadoc

f0c393b

Signed-off-by: Attila Mészáros <[email protected]>

wip

1765745

Signed-off-by: Attila Mészáros <[email protected]>

csviri force-pushed the all-event-mode branch from c1f2daa to 1765745 Compare September 17, 2025 11:00

csviri added 3 commits September 17, 2025 13:14

wip

557012f

Signed-off-by: Attila Mészáros <[email protected]>

wip

d79ee39

Signed-off-by: Attila Mészáros <[email protected]>

wip

5856e3e

Signed-off-by: Attila Mészáros <[email protected]>

csviri marked this pull request as ready for review September 17, 2025 11:39

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 17, 2025

csviri added 2 commits September 17, 2025 13:58

utils integration test

54109e8

Signed-off-by: Attila Mészáros <[email protected]>

test

c5c96b3

Signed-off-by: Attila Mészáros <[email protected]>

metacosm requested changes Sep 23, 2025

View reviewed changes

feat: option to triggering reconciler on all events #2894

Are you sure you want to change the base?

feat: option to triggering reconciler on all events #2894

Uh oh!

Conversation

csviri commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

csviri commented Aug 21, 2025

Uh oh!

csviri commented Sep 17, 2025

Uh oh!

metacosm commented Sep 17, 2025

Uh oh!

csviri commented Sep 17, 2025

Uh oh!

csviri commented Sep 17, 2025

Uh oh!

metacosm commented Sep 17, 2025

Uh oh!

csviri commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

csviri commented Sep 17, 2025

Uh oh!

csviri commented Sep 19, 2025

Uh oh!

csviri commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

csviri commented Aug 11, 2025 •

edited

Loading

csviri commented Sep 17, 2025 •

edited

Loading